✨ Add tiered logging and HTTP request helper#31
Merged
Merged
Conversation
- New `logging` module with `warn!`/`info!`/`debug!` macros driven by the existing `verbose: u8` flag (0=silent, 1=warn, 2=info, 3=debug). Zero-overhead at default verbosity: macros short-circuit before `format!` and the HTTP helper skips timing/body reads. - Live mode buffers logs in a 1000-line ring buffer and flushes them to stderr via an RAII `CaptureGuard` after the alt-screen is restored, so log lines never corrupt the UI. - New `weather::http::get_json` centralizes HTTP send + JSON parse with request observability (INFO: provider/op/status/duration; DEBUG: full URL and truncated response body). All seven providers and the OpenUV helper migrated onto it. - Replace ad-hoc `eprintln!` in `app.rs` and `openuv.rs` with `warn!`. Add deduped missing-translation warnings (one per key per process) and cache hit/miss + enrich-action info logs. - Fix latent bug: a transient OpenUV failure no longer aborts the weather fetch; the error is logged as a warning and `uv_index` stays `None`. `enrich` now returns `()` since it can no longer fail. - README and CHANGELOG updated with `-v`/`-vv`/`-vvv` usage and the new user-visible behavior.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a tiered diagnostic logging system and refactors HTTP request handling across all weather providers to use a centralized helper with integrated logging. It also improves error handling for OpenUV and adds deduplicated logging for missing translation keys. The feedback highlights several uses of 'let chains' in src/logging.rs and src/weather/http.rs, which is an unstable Rust feature; these should be refactored into nested if let statements to maintain compatibility with the stable compiler.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
loggingmodule withwarn!/info!/debug!macros driven by the existingverbose: u8flag (0=silent,1=warn,2=info,3=debug). Zero overhead at the default level — macros short-circuit beforeformat!and the HTTP helper skips timing/body reads. No new crate dependencies.crate::weather::http::get_jsoncentralizes HTTP send + JSON parse for every provider and the OpenUV helper, emitting one INFO line per request (provider, operation, status, latency) and full URLs + truncated bodies at DEBUG.CaptureGuardafter the alt-screen is restored, so the UI stays clean during the session and nothing is lost onq/Esc/Ctrl+Cor panic.eprintln!inapp.rsandopenuv.rs, dedupes missing-translation warnings (one per key per process), and adds cache hit/miss + enrichment-action info logs.-vwarning; the rest of the data still displays.enrichnow returns()since it can no longer fail.CLI usage:
rustormy -c London -v(warnings),-vv(request summaries + cache hits/misses),-vvv(full URLs + bodies). README and CHANGELOG updated with the user-visible behavior.Test plan
just check— fmt + cargo check + 158 tests + clippy pedantic, all greenrustormy -c <city> 2>/tmp/err—/tmp/erris empty-v/-vv/-vvv) with the right info; INFO has no URLs/keys, DEBUG has both[WARN] Provider TomorrowIo failed: ...then succeeds via next provideropen_uvkey triggers[WARN] OpenUV API error: ...and the rest of the weather still renderscache miss, second run logscache hitand skips geocodingrustormy -c <city> -o json -vv 2>/dev/null | jq .parses successfully(capture buffer overflow: N earlier lines dropped)) appears when ring buffer wrapstimecomparison betweenverbose=0andverbose=3shows no measurable difference (HTTP latency dominates)